Add support for % in rgb and fix parsing for % in alpha. #2540
Conversation
|
Hi! I was wondering if this could be merged or if I need to change anything/add more tests to it? |
| ctx.fillStyle = 'rgb( 100%, 99%, 80% 50%)' | ||
| assert.equal('rgba(255, 253, 204, 0.50)', ctx.fillStyle) | ||
|
|
||
| ctx.fillStyle = 'rgb( 100%, 99.99%, 40% / 50%)' |
There was a problem hiding this comment.
To my knowledge, this syntax is not supported in CSS or browsers and requires wrapping with calc().
Or use spaces to separate them:
rgb( 100% 99.99% 40% / 50% )
There was a problem hiding this comment.
To my understanding, the slash here is to separate the rgb from alpha. According to CSS specifications the commas are legacy format, but there is no mention of it being unsupported or deprecated.
https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/color_value/rgb
There was a problem hiding this comment.
I checked the MDN for color syntax, and yeah this makes sense. You cannot mix commas and / in the color property, however this was already the case in node-canvas, I just added a test for this.
@zbjornson @chearon is this something that should be fixed by adding formal syntax validation to rgb (and eventually other type) parsing?
There was a problem hiding this comment.
It would be better not to, but it's probably not a big deal to parse the commas with the slash, since users will almost for sure be using one of the two syntaxes. Good to point it out though.
3 participants
The parsing for % values in alpha wasn't working properly for single digit integers and for all decimal percentages starting with 0. Also added support for % parsing in rgb and rgba. This was mentioned in issue #1391 too.